Skip to content

Fix empty target_config in apply_rust_config bootstrap #144126

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Fix empty target_config in apply_rust_config bootstrap #144126

wants to merge 3 commits into from
+45 −4

Conversation

Shourya742
Copy link
Contributor

This PR fixes the issue of an empty target_config in apply_rust_config, which was caused by the ordering of TOML config parsing. This was inadvertently introduced during the last config refactor. The test and the corresponding configuration order have been corrected in this PR.

r? @Kobzol

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Jul 18, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jul 18, 2025

This PR modifies src/bootstrap/src/core/config.

If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
Kobzol
Kobzol reviewed Jul 18, 2025
View reviewed changes
Kobzol
Kobzol reviewed Jul 21, 2025
View reviewed changes
@Shourya742 Shourya742 force-pushed the 2025-06-18-fix-target-config-issue branch 2 times, most recently from 40c6ae8 to 97b165f Compare July 21, 2025 08:49
@Shourya742 Shourya742 requested a review from Kobzol July 21, 2025 09:17
Kobzol
Kobzol reviewed Jul 21, 2025
View reviewed changes
Copy link
Member

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Left some suggestions, but the general shape is solid.

@Shourya742
add default_lld_opt_in_targets method and modified test accordingly
4ca5afd 
add test for lld opt in and also add thread_local defined state to change opt in targets

make the config lld test parameter smoother to work, and have no_llvm_config set even when target_config is not present
@Shourya742
add test_lld_opt_in test
fb10084
@Shourya742 Shourya742 force-pushed the 2025-06-18-fix-target-config-issue branch from d85be56 to fb10084 Compare July 21, 2025 14:44
@Shourya742 Shourya742 requested a review from Kobzol July 21, 2025 15:16
@Kobzol
Copy link
Member

Kobzol commented Jul 21, 2025

@bors r+

@bors
Copy link
Collaborator

bors commented Jul 21, 2025

📌 Commit fb10084 has been approved by Kobzol

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kobzol Kobzol Awaiting requested review from Kobzol

Assignees

@Kobzol Kobzol

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Shourya742 @rustbot @rust-log-analyzer @Kobzol @bors